Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(gear-builtin): Separate actor id declaration #4051

Merged
merged 5 commits into from
Aug 13, 2024
Merged

feat(gear-builtin): Separate actor id declaration #4051

merged 5 commits into from
Aug 13, 2024

Conversation

holykol
Copy link
Contributor

@holykol holykol commented Jul 11, 2024

  • move ID out of BuiltinActor trait
  • introduce ActorWithId struct one can use to assign an id to builtin at compile time
  • simplify some tests as a result

Resolves #4049

holykol added 2 commits July 11, 2024 23:08
@holykol
feat(gear-builtin): Separate actor id declaration
4e26e60 
* move ID out of BuiltinActor trait
* introduce ActorWithId struct one can use to assign an id to builtin at compile
  time
* simplify some tests as a result
@holykol
correct comment
34543a9
@holykol holykol requested a review from breathx July 12, 2024 12:39
@holykol holykol self-assigned this Jul 12, 2024
@holykol holykol marked this pull request as ready for review July 12, 2024 12:39
@holykol holykol requested a review from breathx July 15, 2024 10:01
@ekovalev
Copy link
Member

ekovalev commented Aug 10, 2024 •
edited
Loading

Please convince me this is actually a good thing ).

In any case an actor type id is assigned at compile time, and we have a whole u64 type to make sure every builtin actor we will ever want to introduce has its unique ID assigned once for all runtimes it appears in. The latter is important if we recall that this type ID determines the 32-byte ActorID address which is then published and used in programs. This way the contracts developers could rely on the fact a builtin's address persists across all runtimes and wouldn't have to write different versions of the same program for different chains.

Or am I missing the point?

@breathx
Copy link
Member

breathx commented Aug 11, 2024

Please convince me this is actually a good thing ).

In any case an actor type id is assigned at compile time, and we have a whole u64 type to make sure every builtin actor we will ever want to introduce has its unique ID assigned once for all runtimes it appears in. The latter is important if we recall that this type ID determines the 32-byte ActorID address which is then published and used in programs. This way the contracts developers could rely on the fact a builtin's address persists across all runtimes and wouldn't have to write different versions of the same program for different chains.

Or am I missing the point?

Discussed personally

@breathx breathx assigned breathx and unassigned holykol Aug 11, 2024
@breathx breathx added the A2-mergeoncegreen PR is ready to merge after CI passes label Aug 12, 2024
@breathx breathx merged commit c0634e2 into master Aug 13, 2024
14 checks passed
@breathx breathx deleted the holykol/resolve-4049 branch August 13, 2024 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@breathx breathx breathx approved these changes

@ekovalev ekovalev ekovalev approved these changes

Assignees

@breathx breathx

Labels
A2-mergeoncegreen PR is ready to merge after CI passes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Builtin actor id declaration
3 participants
@holykol @ekovalev @breathx